Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #7220: resolve global esc key listener conflict in Calendar component #7228

Merged
merged 3 commits into from
Sep 23, 2024

Conversation

ozantekin
Copy link
Contributor

@ozantekin ozantekin commented Sep 21, 2024

Fix #7220

This also resolves the following issues:

  • Fixed the same error in the Time section on the Calendar page.
    Screen Shot 2024-09-21 at 23 09 10 PM
  • Corrected the overlay behavior that did not open and close properly when the target was not correctly identified.

Copy link

vercel bot commented Sep 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Sep 23, 2024 6:21am
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Sep 23, 2024 6:21am

@@ -1852,7 +1854,7 @@ export const Calendar = React.memo(
setOverlayVisibleState(true);

overlayEventListener.current = (e) => {
if (!isOutsideClicked(e)) {
if (!isOutsideClicked(e.target)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line looks incorrect as isOutsideClicked takes an event and gets its event.target?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When target is not included, the panel of the first calendar input stays open after making changes in the first input and shifting focus to the second calendar input.

Screen Shot 2024-09-22 at 15 01 01 PM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK but if you look at the code

const isOutsideClicked = (event) => {
            return elementRef.current && !(elementRef.current.isSameNode(event.target) || isNavIconClicked(event.target) || elementRef.current.contains(event.target) || (overlayRef.current && overlayRef.current.contains(event.target)));
        };

That means if you pass e.target then inside isOutSideClicked they will be calling target.target does that make sense? the Target is a DOM element and it won't have a target property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification, I missed that detail. I'll remove the target and push the update. Can I address the other issue in a different PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can keep modifying this PR its OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noticed missing condition, added a fix to resolve the issue.

@melloware melloware merged commit 7265aeb into primefaces:master Sep 23, 2024
5 checks passed
@mrpsiho
Copy link

mrpsiho commented Oct 1, 2024

I am using primereact version 10.8.3 and the error persists Uncaught Error: Unexpected: global esc key listener with priority [600, 0] already exists.. It can be reproduced also in your demo. Steps to reproduce:

  1. Go to https://primereact.org/calendar/#time
  2. Open "Time Only" picker and change hour to any other value. Just click up or down to change it. Specifically hours! Changing minutes seems to works fine.
  3. Click on the sibling datepicker/timepicker. The application crashes.
    When clicking Esc or non-datepicker element - it works though.

Maybe better to create a new task, right?

@ozantekin
Copy link
Contributor Author

I am using primereact version 10.8.3 and the error persists Uncaught Error: Unexpected: global esc key listener with priority [600, 0] already exists.. It can be reproduced also in your demo. Steps to reproduce:

  1. Go to https://primereact.org/calendar/#time
  2. Open "Time Only" picker and change hour to any other value. Just click up or down to change it. Specifically hours! Changing minutes seems to works fine.
  3. Click on the sibling datepicker/timepicker. The application crashes.
    When clicking Esc or non-datepicker element - it works though.

Maybe better to create a new task, right?

I've fixed this issue, but it hasn't been pushed to production yet. You can track further details here. #7220 @mrpsiho

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calendar: Global esc key listener with priority [600, 0] already exists
3 participants